Skip to content

Conversation

@tvarohohlavy
Copy link
Contributor

@tvarohohlavy tvarohohlavy commented Jan 5, 2026

Summary

Extends secret placeholder support (env://VAR, file://name, encv1:...) to all configuration fields across volumes, repositories, and notifications.

Changes

Core

  • Added resolveSecretsDeep<T>() to crypto.ts - recursively resolves all string values in objects/arrays with circular reference protection

Restic Operations

  • Added resolveAndBuild() helper that resolves config secrets once per operation
  • Removed individual resolveSecret() calls from buildEnv()
  • Applied to all 12 restic operations (init, backup, restore, snapshots, forget, etc.)

Volume Backends

  • Refactored backend.ts dispatcher to resolve secrets at mount time only
  • Removed redundant resolveSecret() calls from SMB, WebDAV, and SFTP backends

Notifications

  • Removed 49-line decryptSensitiveFields switch statement
  • Inlined resolveSecretsDeep() at call sites

Backend Compatibility

  • Updated credential comparison to resolve both configs upfront

Benefits

  • Users can now use env://MY_VAR or file://secret-name in any config field
  • Reduced code duplication across backends
  • Single point of secret resolution per operation

Summary by CodeRabbit

  • New Features

    • Secret references now resolve dynamically for any configuration field (repositories, volumes, notifications, etc.).
    • Added env://VAR_NAME and file://SECRET_NAME reference formats for environment and Docker Secrets.
  • Documentation

    • Clarified examples and wording to show broader applicability (passwords, keys, webhook URLs, etc.) and updated guidance on secret references.

✏️ Tip: You can customize this high-level summary in your review settings.

- Add resolveSecretsDeep() to recursively resolve secret placeholders
  (env://, file://, encv1:) in any config field
- Refactor restic operations to resolve secrets once via resolveAndBuild()
- Refactor volume backend dispatcher to resolve secrets at mount time
- Remove redundant per-field resolveSecret() calls from SMB, WebDAV, SFTP backends
- Simplify notifications service by replacing switch-based decryption
- Update backend-compatibility to use deep resolution for credential comparison

This allows users to use secret references in any configuration field,
not just the designated sensitive fields.
Copilot AI review requested due to automatic review settings January 5, 2026 16:44
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Centralizes secret reference resolution by adding resolveSecretsDeep and resolving secrets before use across backends, notifications, and restic operations; removes per-backend runtime secret decryption and updates documentation to state secret placeholders are valid in any configuration field. (≈36 words)

Changes

Cohort / File(s) Summary
Documentation
README.md, examples/secrets-placeholders/README.md
Clarified secret reference semantics: env://VAR_NAME and file://SECRET_NAME apply to any configuration field; updated examples and wording.
Secret resolution utility
app/server/utils/crypto.ts
Added resolveSecretsDeep<T>(input: T): Promise<T> to recursively resolve secret references in objects/arrays, handling circular refs via WeakMap; exported on cryptoUtils.
Backend factory & orchestration
app/server/modules/backends/backend.ts
Introduced getBackendFactory and refactored backend instantiation to call cryptoUtils.resolveSecretsDeep on mount path before creating/mounting backends; unmount/checkHealth use original config.
Backend implementations
app/server/modules/backends/sftp/sftp-backend.ts, app/server/modules/backends/smb/smb-backend.ts, app/server/modules/backends/webdav/webdav-backend.ts
Removed per-backend runtime secret resolution and cryptoUtils imports; backends now use config.* values directly (passwords/private keys passed without decrypting), with minor normalization retained for private keys.
Notifications
app/server/modules/notifications/notifications.service.ts
Replaced local decryptSensitiveFields with cryptoUtils.resolveSecretsDeep(destination.config) (renamed to resolvedConfig) when building Shoutrrr URLs for testDestination and backup notification flows.
Compatibility & restic utilities
app/server/utils/backend-compatibility.ts, app/server/utils/restic.ts
backend-compatibility.ts: resolves both configs up-front and compares resolved credentials. restic.ts: introduced resolveAndBuild(config) to resolve secrets and build repo URL/env; removed per-field runtime decrypt calls and consolidated SFTP/CA/RESTIC env handling.
Other
package.json
Manifest touched (dependency/manifest line changes indicated).

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: Support secret placeholders in all config fields' accurately reflects the main change—extending secret placeholder support across all configuration fields, which is the primary objective of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends secret placeholder support (env://VAR, file://name, encv1:...) from specific sensitive fields to all configuration fields across volumes, repositories, and notifications. The refactoring centralizes secret resolution through a new resolveSecretsDeep() utility that recursively processes entire configuration objects.

Key changes:

  • Added deep recursive secret resolution supporting nested objects and arrays with circular reference protection
  • Introduced resolveAndBuild() helper in restic operations to resolve secrets once per operation
  • Refactored backend mount logic to resolve secrets at mount time only

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/server/utils/crypto.ts Added resolveSecretsDeep() function with recursive traversal and isPlainObject() helper for type checking
app/server/utils/restic.ts Removed individual resolveSecret() calls from buildEnv(), added resolveAndBuild() helper, and applied it across all 12 restic operations
app/server/modules/backends/backend.ts Refactored dispatcher to resolve secrets once at mount time via wrapper functions
app/server/modules/backends/smb/smb-backend.ts Removed local resolveSecret() call for password (now resolved in dispatcher)
app/server/modules/backends/webdav/webdav-backend.ts Removed local resolveSecret() call for password (now resolved in dispatcher)
app/server/modules/backends/sftp/sftp-backend.ts Removed local resolveSecret() calls for password and private key (now resolved in dispatcher)
app/server/modules/notifications/notifications.service.ts Replaced 49-line decryptSensitiveFields() switch statement with inline resolveSecretsDeep() calls
app/server/utils/backend-compatibility.ts Updated credential comparison to resolve both configs upfront before comparison
README.md Updated documentation to clarify that secret placeholders work in "any configuration field"
examples/secrets-placeholders/README.md Updated example documentation to reflect the expanded scope

* Helper to check if a value is a plain object (not null, array, Date, etc.)
*/
const isPlainObject = (value: unknown): value is Record<string, unknown> => {
return typeof value === "object" && value !== null && !Array.isArray(value) && value.constructor === Object;
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isPlainObject check uses value.constructor === Object, which may fail for objects created with Object.create(null) or objects from different realms/contexts. Consider using a more robust check like Object.prototype.toString.call(value) === '[object Object]' or simply checking if the constructor exists before comparing it.

Suggested change
return typeof value === "object" && value !== null && !Array.isArray(value) && value.constructor === Object;
if (typeof value !== "object" || value === null) return false;
if (Object.prototype.toString.call(value) !== "[object Object]") return false;
const proto = Object.getPrototypeOf(value);
return proto === Object.prototype || proto === null;

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In @app/server/modules/backends/smb/smb-backend.ts:
- Line 42: unmount() and checkHealth() instantiate the SMB backend with
unresolved credential placeholders because they pass volume.config directly; fix
by resolving secrets via cryptoUtils.resolveSecretsDeep(volume.config) before
calling makeBackend in both unmount and checkHealth (mirror what mount does),
i.e., call const resolvedConfig = await
cryptoUtils.resolveSecretsDeep(volume.config) and then use
makeBackend(resolvedConfig, path).unmount()/checkHealth().
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21040d7 and bfbbea3.

📒 Files selected for processing (10)
  • README.md
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/modules/notifications/notifications.service.ts
  • app/server/utils/backend-compatibility.ts
  • app/server/utils/crypto.ts
  • app/server/utils/restic.ts
  • examples/secrets-placeholders/README.md
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use Biome for code formatting and linting with bunx biome check --write ., format only with bunx biome format --write ., or lint with bunx biome lint .

Files:

  • app/server/utils/backend-compatibility.ts
  • app/server/utils/crypto.ts
  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/notifications/notifications.service.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses "type": "module"

Files:

  • app/server/utils/backend-compatibility.ts
  • app/server/utils/crypto.ts
  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/notifications/notifications.service.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
app/server/modules/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Server follows a modular service-oriented architecture with controller-service-database pattern in each module

Files:

  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/notifications/notifications.service.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
app/server/modules/backends/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

app/server/modules/backends/**/*.ts: Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools
Volume mounts are stored in /var/lib/zerobyte/mounts/<volume-name>

Files:

  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
app/server/utils/restic.ts

📄 CodeRabbit inference engine (AGENTS.md)

app/server/utils/restic.ts: Restic password file must have 0600 permissions and should never be exposed
Update buildRepoUrl() in app/server/utils/restic.ts when adding a new repository backend
Update buildEnv() in app/server/utils/restic.ts to handle credentials and configuration when adding a new repository backend
Restic password file is stored in /var/lib/zerobyte/restic/password and auto-generated on first run
Restic cache is stored in /var/lib/zerobyte/restic/cache

Files:

  • app/server/utils/restic.ts
app/server/modules/backends/backend.ts

📄 CodeRabbit inference engine (AGENTS.md)

Update backend factory in app/server/modules/backends/backend.ts when adding a new volume backend

Files:

  • app/server/modules/backends/backend.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend
📚 Learning: 2025-12-01T22:25:20.502Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 92
File: app/client/components/export-dialog.tsx:189-196
Timestamp: 2025-12-01T22:25:20.502Z
Learning: Volume secrets in ZeroByte are not yet encrypted in the database (stored as cleartext) and are excluded from the export dialog's secrets handling. The export logic relies on the `isEncrypted()` helper function that checks for the "encv1" encryption prefix, so cleartext volume secrets cannot be automatically detected for exclusion. Once volume secret encryption is implemented, the export logic for volumes will need to be completed to enable proper secrets handling options.

Applied to files:

  • README.md
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend

Applied to files:

  • app/server/utils/backend-compatibility.ts
  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/backend.ts : Update backend factory in `app/server/modules/backends/backend.ts` when adding a new volume backend

Applied to files:

  • app/server/utils/backend-compatibility.ts
  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/**/*.ts : Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools

Applied to files:

  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/modules/backends/backend.ts
  • app/server/modules/backends/sftp/sftp-backend.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run

Applied to files:

  • app/server/modules/backends/smb/smb-backend.ts
  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
📚 Learning: 2025-11-30T15:18:38.913Z
Learnt from: tvarohohlavy
Repo: nicotsx/zerobyte PR: 89
File: app/server/modules/notifications/builders/telegram.ts:3-8
Timestamp: 2025-11-30T15:18:38.913Z
Learning: In app/server/modules/notifications/builders/telegram.ts, the Telegram bot token in Shoutrrr URLs must NOT be URL-encoded. When passed to the Shoutrrr CLI, encoding the bot token (e.g., encoding the colon as %3A) causes "failed to initialize service" errors. The token should be used raw in the URL userinfo section. Chat IDs in the channels query parameter can optionally be encoded but work fine unencoded as well.

Applied to files:

  • app/server/modules/notifications/notifications.service.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend

Applied to files:

  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
  • app/server/modules/backends/backend.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and should never be exposed

Applied to files:

  • app/server/modules/backends/webdav/webdav-backend.ts
  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic cache is stored in `/var/lib/zerobyte/restic/cache`

Applied to files:

  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/**/*.ts : Volume mounts are stored in `/var/lib/zerobyte/mounts/<volume-name>`

Applied to files:

  • app/server/modules/backends/backend.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/schemas/volumes.ts : Add schemas for new volume backends to `app/schemas/volumes.ts` and update `volumeConfigSchema` discriminated union

Applied to files:

  • app/server/modules/backends/backend.ts
🧬 Code graph analysis (5)
app/server/utils/backend-compatibility.ts (1)
app/server/utils/crypto.ts (1)
  • cryptoUtils (231-235)
app/server/modules/backends/smb/smb-backend.ts (1)
app/server/core/config.ts (1)
  • config (57-57)
app/server/modules/notifications/notifications.service.ts (2)
app/server/utils/crypto.ts (1)
  • cryptoUtils (231-235)
app/server/modules/notifications/builders/index.ts (1)
  • buildShoutrrrUrl (12-37)
app/server/utils/restic.ts (4)
app/server/core/config.ts (1)
  • config (57-57)
app/server/core/constants.ts (1)
  • RESTIC_PASS_FILE (5-5)
app/schemas/restic.ts (1)
  • RepositoryConfig (97-97)
app/server/utils/crypto.ts (1)
  • cryptoUtils (231-235)
app/server/modules/backends/sftp/sftp-backend.ts (1)
app/server/core/config.ts (1)
  • config (57-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Agent
🔇 Additional comments (20)
examples/secrets-placeholders/README.md (1)

3-3: LGTM!

Documentation accurately reflects the expanded secret placeholder support introduced by resolveSecretsDeep.

app/server/utils/crypto.ts (3)

186-191: LGTM!

The isPlainObject helper correctly distinguishes plain objects from arrays, null, and class instances using the constructor === Object check.


193-229: LGTM!

The recursive secret resolver is well-implemented:

  • WeakMap correctly handles circular references by storing results before recursing
  • Properly delegates string resolution to resolveSecret
  • Correctly traverses arrays and plain objects
  • Returns non-object primitives unchanged

231-235: LGTM!

Export correctly exposes the new resolveSecretsDeep utility alongside existing functions.

app/server/modules/notifications/notifications.service.ts (2)

179-181: LGTM!

Correctly replaces per-field decryption with resolveSecretsDeep. The resolved config is properly passed to buildShoutrrrUrl. Based on learnings, the Telegram bot token will be passed raw (not URL-encoded), which is the required behavior.


281-282: LGTM!

Secret resolution within the notification loop is appropriate here. Each destination may have different encrypted/referenced secrets that need individual resolution.

README.md (1)

89-94: LGTM!

Documentation clearly explains the expanded secret reference support, consistent with the resolveSecretsDeep implementation.

app/server/utils/backend-compatibility.ts (2)

38-51: LGTM!

Resolving secrets upfront before credential comparison is the correct approach. This ensures encrypted/referenced secrets are properly compared by their actual values rather than their stored representations.


55-88: LGTM!

All backend credential comparisons (GCS, Azure, REST) correctly use the pre-resolved configurations. The logic for handling optional REST credentials (username/password) is preserved.

app/server/modules/backends/webdav/webdav-backend.ts (1)

50-54: Secrets are correctly resolved before backend instantiation.

The WebDAV backend receives config.password from the resolved configuration. In backend.ts, the mount operation calls resolveSecretsDeep on line 46, then passes the resolved config to makeBackend() on line 47, ensuring credentials are decrypted before the backend uses them to write to the secrets file.

app/server/modules/backends/sftp/sftp-backend.ts (2)

80-87: LGTM! Secret resolution moved to dispatcher.

The removal of runtime secret resolution is correct. The config.privateKey is now resolved by the dispatcher in backend.ts (line 46) before the backend's mount() is called, so this code correctly assumes it receives a plain private key value.


95-102: LGTM! Password handling is correct and secure.

The password is now resolved by the dispatcher in backend.ts before mount, so using config.password directly is correct. The password is securely passed via stdin (not exposed in process arguments), which is the proper approach.

app/server/modules/backends/backend.ts (3)

23-38: LGTM! Clean refactor to factory pattern.

The extraction of backend selection into getBackendFactory improves modularity and makes the code easier to test and maintain. All backend types are properly mapped.


49-50: LGTM! Appropriate optimization for unmount/checkHealth.

Using the unresolved config for unmount() and checkHealth() is correct and efficient. These operations only interact with the local mount point and don't require authentication to remote systems, so avoiding secret resolution is a good optimization.


44-48: Add tests for circular reference handling in resolveSecretsDeep.

The circular reference protection is correctly implemented using a WeakMap to track visited objects and arrays—it checks seen.has(value) before processing and stores intermediate results to prevent infinite loops. However, no tests were found for this functionality. Add test cases for circular reference scenarios to ensure the protection works reliably in production.

⛔ Skipped due to learnings
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/**/*.ts : Each volume backend (NFS, SMB, WebDAV, SFTP, directory) must implement mounting logic using system tools
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/schemas/volumes.ts : Add schemas for new volume backends to `app/schemas/volumes.ts` and update `volumeConfigSchema` discriminated union
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/**/*.ts : Volume mounts are stored in `/var/lib/zerobyte/mounts/<volume-name>`
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/modules/backends/backend.ts : Update backend factory in `app/server/modules/backends/backend.ts` when adding a new volume backend
app/server/utils/restic.ts (5)

124-133: LGTM! S3/R2 credentials now resolved upfront.

The removal of per-field resolveSecret() calls is correct. Credentials are now resolved by resolveAndBuild() before buildEnv() is called, so direct field access is appropriate.


134-140: LGTM! GCS credentials properly secured.

The GCS credentials are now resolved upfront and written with secure permissions (0o600). The temporary credentials file is properly cleaned up by cleanupTemporaryKeys() (lines 882-884).


158-202: LGTM! SFTP key handling is secure and correct.

The SFTP private key handling properly:

  • Receives resolved key from resolveAndBuild()
  • Normalizes line endings without decryption
  • Detects and rejects encrypted keys (lines 166-169)
  • Writes with secure permissions (0o600)
  • Cleans up temporary files via cleanupTemporaryKeys() (lines 870-872)

812-867: LGTM! Copy operation properly handles dual configurations.

The copy() function correctly:

  • Resolves secrets for both source and destination repos (lines 820-821)
  • Merges environments appropriately (lines 823-827)
  • Preserves the resolved source config for SFTP backend detection (line 843)
  • Cleans up temporary files from both configurations (lines 852-853)

The dual-config handling is well-structured and consistent with the single-config pattern used in other operations.


218-227: LGTM! Excellent refactor to centralize secret resolution.

The resolveAndBuild() helper consolidates the pattern of resolving secrets, building the repo URL, and constructing the environment. This ensures all operations follow the same approach and reduces code duplication. Verification confirms all 11 restic operations consistently use this helper, with no direct calls to buildRepoUrl() or buildEnv() that bypass secret resolution.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/server/utils/restic.ts (1)

108-216: Fix migration.ts to resolve secrets before calling buildEnv and buildRepoUrl.

After the refactor, buildEnv and buildRepoUrl no longer call resolveSecretsDeep inline—they expect pre-resolved credentials. However, app/server/modules/lifecycle/migration.ts at lines 64-65 calls these functions directly with repository.config, which contains encrypted secrets. This will cause the migration to fail since encrypted values will be used as actual credentials.

Either export resolveAndBuild from restic.ts and use it in migration.ts, or resolve secrets in migration.ts before calling these functions:

const resolved = await cryptoUtils.resolveSecretsDeep(repository.config);
const repoUrl = buildRepoUrl(resolved);
const env = await buildEnv(resolved);
🧹 Nitpick comments (1)
app/server/utils/crypto.ts (1)

189-194: Edge case: objects with null prototype are excluded.

Objects created via Object.create(null) have no constructor and won't be traversed. This is unlikely to occur with JSON-parsed configs, but worth noting.

♻️ Optional: Handle null-prototype objects
 const isPlainObject = (value: unknown): value is Record<string, unknown> => {
-	return typeof value === "object" && value !== null && !Array.isArray(value) && value.constructor === Object;
+	return typeof value === "object" && value !== null && !Array.isArray(value) && 
+		(value.constructor === Object || Object.getPrototypeOf(value) === null);
 };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfbbea3 and 3d87bfa.

📒 Files selected for processing (4)
  • README.md
  • app/server/modules/notifications/notifications.service.ts
  • app/server/utils/crypto.ts
  • app/server/utils/restic.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/server/modules/notifications/notifications.service.ts
  • README.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use Biome for code formatting and linting with bunx biome check --write ., format only with bunx biome format --write ., or lint with bunx biome lint .

Files:

  • app/server/utils/crypto.ts
  • app/server/utils/restic.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use tabs (not spaces) for indentation with a line width of 120 characters
Use double quotes for strings
Do not auto-organize imports - imports organization is disabled in Biome
All imports must include file extensions when targeting Node/Bun, as the project uses "type": "module"

Files:

  • app/server/utils/crypto.ts
  • app/server/utils/restic.ts
app/server/utils/restic.ts

📄 CodeRabbit inference engine (AGENTS.md)

app/server/utils/restic.ts: Restic password file must have 0600 permissions and should never be exposed
Update buildRepoUrl() in app/server/utils/restic.ts when adding a new repository backend
Update buildEnv() in app/server/utils/restic.ts to handle credentials and configuration when adding a new repository backend
Restic password file is stored in /var/lib/zerobyte/restic/password and auto-generated on first run
Restic cache is stored in /var/lib/zerobyte/restic/cache

Files:

  • app/server/utils/restic.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildEnv()` in `app/server/utils/restic.ts` to handle credentials and configuration when adding a new repository backend

Applied to files:

  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Update `buildRepoUrl()` in `app/server/utils/restic.ts` when adding a new repository backend

Applied to files:

  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file is stored in `/var/lib/zerobyte/restic/password` and auto-generated on first run

Applied to files:

  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic password file must have 0600 permissions and should never be exposed

Applied to files:

  • app/server/utils/restic.ts
📚 Learning: 2025-12-28T17:31:39.171Z
Learnt from: CR
Repo: nicotsx/zerobyte PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T17:31:39.171Z
Learning: Applies to app/server/utils/restic.ts : Restic cache is stored in `/var/lib/zerobyte/restic/cache`

Applied to files:

  • app/server/utils/restic.ts
🧬 Code graph analysis (1)
app/server/utils/restic.ts (4)
app/server/core/config.ts (1)
  • config (57-57)
app/server/core/constants.ts (1)
  • RESTIC_PASS_FILE (5-5)
app/schemas/restic.ts (1)
  • RepositoryConfig (97-97)
app/server/utils/crypto.ts (1)
  • cryptoUtils (241-246)
🔇 Additional comments (6)
app/server/utils/crypto.ts (2)

196-232: LGTM!

The circular reference handling via WeakMap is correctly implemented—setting the result object before recursing ensures cycles resolve properly. Sequential processing of arrays is a reasonable choice to avoid overwhelming external secret sources.


241-246: LGTM!

The new function is correctly exposed alongside existing crypto utilities.

app/server/utils/restic.ts (4)

218-227: LGTM!

Clean consolidation of secret resolution, URL construction, and environment setup. Returning resolved enables downstream checks (e.g., checking backend type in copy).

Based on learnings, this aligns with the guideline to update buildEnv for handling credentials when adding new backends—the resolution now happens upstream but the pattern remains consistent.


229-249: LGTM!

The init operation correctly uses the new resolveAndBuild pattern with proper cleanup.


812-867: LGTM!

The copy operation correctly resolves both source and destination configs separately. The env merge with destEnv overriding sourceEnv is intentional, with RESTIC_FROM_PASSWORD_FILE explicitly preserving the source password.

Note: If both source and dest are SFTP backends, _SFTP_KEY_PATH from source will be overwritten by dest. This is fine since lines 843-845 conditionally add source SFTP args, and the dest key path is used by default restic behavior.


429-429: LGTM!

All remaining operations (restore, snapshots, forget, deleteSnapshots, tagSnapshots, ls, unlock, check, repairIndex) consistently use resolveAndBuild with proper cleanup.

Also applies to: 511-511, 542-542, 583-583, 612-612, 679-679, 734-734, 752-752, 789-789

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant